Skip to content

Fix #1027: clear stuck "Needs input" when the follow-up event turnId drifts - #5666

Open
RubiconPerform wants to merge 4 commits into
manaflow-ai:mainfrom
RubiconPerform:agent/fix-1027-stuck-needsinput
Open

RubiconPerform wants to merge 4 commits into
manaflow-ai:mainfrom
RubiconPerform:agent/fix-1027-stuck-needsinput

Conversation

@RubiconPerform

@RubiconPerform RubiconPerform commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

What changed? Fixes #1027 — the sidebar agent status getting stuck on "Needs input".

Root cause. When Claude fires a Notification, the CLI sets the session lifecycle to needsInput but does not advance the active turn (it stays pointing at the prior prompt's turn). The events that should later clear it — stop (→ idle), prompt-submit (→ running), pre-tool-use (→ running) — are all gated by ClaudeHookSessionStore.isCurrent(...), which requires the event's turnId and sessionId to match the workspace's active turn. On a resume / turn drift the follow-up event carries a different turnId, so isCurrent returns false, the clearing mutation is dropped, and the badge is stranded on "Needs input". It's intermittent precisely because it depends on whether the turn ids happen to line up.

Fix. New ClaudeHookSessionStore.isCurrentOrClearsStaleNeedsInput(...), used only by the three Claude clearing handlers (stop, prompt-submit, pre-tool-use). It returns true when isCurrent is true or when the event is for the same active session and that session is currently stuck on .needsInput (ignoring the turnId). Because it still requires active.sessionId == sessionId, a different-session event keeps failing closed — so the protection asserted by testClaudeStopFromPreviousSessionDoesNotClobberClearRunningStatus is preserved. The needsInput-setting paths (Notification, AskUserQuestion) keep the strict isCurrent gate, so this relaxation can only ever clear a stale "Needs input", never spuriously raise one.

Scoped intentionally to the stuck-needsInput root cause. The related "Running persists after stop" and "status pill missing on new workspaces" symptoms are separate and already have in-flight PRs (#5500 / #5662 / #5660), so I kept this minimal and did not touch the app-side aggregation or the generic-agent notification path.

Two files: CLI/cmux.swift (gate + wiring) and cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift (regression tests).

Testing

Three regression tests added to the already-wired CLINotifyProcessIntegrationRegressionTests.swift, each driving the real CLI hook binary:

  • testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftStop — prompt-submit(turn-1) → notification (→ needsInput) → stop(turn-2, same session): asserts the lifecycle clears to idle, emits set_agent_lifecycle claude_code idle, and does not re-assert "Needs input".
  • testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftPreToolUse — same setup, pre-tool-use(turn-2, Bash) → clears to running.
  • testClaudeNotificationNeedsInputStaysStuckForDifferentSessionStop — guardrail: a different-session Stop must not clear the active session's needsInput.

⚠️ Local verification note. My machine only has Xcode 15; this repo needs Xcode 16+/26 (swift-tools-version: 6.0), so I could not run the full xcodebuild -scheme cmux-unit suite locally. What I did verify locally:

  • swiftc -parse clean on both changed files.
  • scripts/lint-pbxproj-test-wiring.sh → ok (the test file is already wired; no new pbxproj entries needed).
  • A standalone harness compiling the verbatim isCurrent + isCurrentOrClearsStaleNeedsInput functions and asserting: same-session turn-drift clears, different-session stays blocked, non-needsInput status is not over-cleared, and same-turn behavior is unchanged — all pass.

Please run the suite on CI to confirm end-to-end:

xcodebuild -scheme cmux-unit -configuration Debug -destination 'platform=macOS' test \
  -only-testing:cmuxTests/CLINotifyProcessIntegrationRegressionTests

Demo Video

N/A — internal status-state logic, covered by the new integration tests.

Checklist

  • I tested the change locally (logic harness + swiftc -parse + pbxproj wiring lint; full xcodebuild suite needs Xcode 26 — see note above)
  • I added tests for the behavior change
  • Docs/changelog — n/a (bugfix)
  • Requesting bot reviews after this commit (comment below)
  • All code review bot comments resolved
  • All human review comments resolved

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Fixes #1027 by clearing a stuck "Needs input" even when the follow-up stop, prompt-submit, or pre-tool-use event has a different turnId. Adds an atomic, same-session clearing gate that preserves cross-session safety, and localizes the Claude "Waiting" subtitle.

  • Bug Fixes
    • Added ClaudeHookSessionStore.isCurrentOrClearsStaleNeedsInput and shouldApplyClaudeHookClearingMutation; applied to stop, prompt-submit, and non-AskUserQuestion pre-tool-use. The gate runs under a single withLockedState and logs with claude-hook.clearing-gate.error.
    • Restructured pre-tool-use so AskUserQuestion is handled first as a needsInput-setting event with the strict gate and an early return; a drifted/stale question can’t clear a live "Needs input". Localized the "Waiting" subtitle via agent.claude.input.subtitle.waiting (en/ja).
    • Added regression tests covering same-session drift clears (stop, prompt-submit, pre-tool-use) and guards that different-session events and drifted AskUserQuestion do not clear the state.

Written for commit 1733504. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of drifted turn IDs so same-session events can clear stale "Needs input" indicators without affecting other sessions; AskUserQuestion no longer overwrites live "Needs input".
  • Tests

    • Added integration tests validating lifecycle clearing and session isolation for drifted-turn scenarios.
  • New Features

    • Added a localized "Waiting" subtitle for Claude agent input (English and Japanese).

…vent's turn id drifts

A Claude `Notification` sets the session lifecycle to `needsInput` but does not
advance the active turn (it stays pointing at the prior prompt's turn). The
follow-up events that should clear it -- `stop` (-> idle), `prompt-submit`
(-> running) and `pre-tool-use` (-> running) -- are all gated by
`ClaudeHookSessionStore.isCurrent(...)`, which requires the event's turnId
(and sessionId) to match the active turn. On a resume / turn drift the
follow-up event carries a different turnId, so `isCurrent` returns false, the
clearing mutation is dropped, and the sidebar is stranded on "Needs input".
It is intermittent because it depends on whether the turn ids line up.

Add `ClaudeHookSessionStore.isCurrentOrClearsStaleNeedsInput(...)` and use it
ONLY in the three Claude clearing transitions. It returns true when
`isCurrent` is true, or when the event belongs to the SAME active session and
that session is currently stuck on `.needsInput` (ignoring the turnId). The
`active.sessionId == sessionId` requirement keeps a DIFFERENT-session event
failing closed, so the protection in
`testClaudeStopFromPreviousSessionDoesNotClobberClearRunningStatus` is
preserved. The needsInput-SETTING paths (`Notification`, `AskUserQuestion`)
keep the strict gate, so this relaxation can only ever clear a stale
"Needs input", never spuriously raise one.

Adds three regression tests that drive the real CLI hooks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown

@RubiconPerform is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e785403-0818-45dc-aaf2-de5bd9480170

📥 Commits

Reviewing files that changed from the base of the PR and between 05d62c5 and 1733504.

📒 Files selected for processing (2)
  • CLI/cmux.swift
  • Resources/Localizable.xcstrings

📝 Walkthrough

Walkthrough

Relax CLEARING eligibility so same-session events with a drifted turnId can clear a stale needsInput, while preserving strict visible/current gating for events that set needsInput. Add five integration tests covering clearing and session isolation.

Changes

Claude needsInput lifecycle clearing

Layer / File(s) Summary
Clearing gate contract and helpers
CLI/cmux.swift
Adds isCurrentOrClearsStaleNeedsInput(...) to permit CLEARING when the event is current or when the normalized active session matches and is stuck on .needsInput. Adds shouldApplyClaudeHookClearingMutation(...) wrapper delegating to the helper, recording a telemetry breadcrumb on error and returning true on exception.
CLEARING transition updates & AskUserQuestion gating
CLI/cmux.swift
Stop→idle, prompt-submit→running, and non-"AskUserQuestion" pre-tool-use→running CLEARING branches now use shouldApplyClaudeHookClearingMutation(...). Reworks pre-tool-use so "AskUserQuestion" runs earlier under strict visible/current gating to avoid clearing a live needsInput.
Integration regression tests for drifted-turn behavior
cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
Adds five tests asserting same-session drifted Stop clears needsInput→idle without re-asserting; same-session drifted PreToolUse clears needsInput→running; drift-clearing is same-session only; drifted prompt-submit clears needsInput→running; and drifted AskUserQuestion must not clear a live needsInput.
Localization addition
Resources/Localizable.xcstrings
Adds agent.claude.input.subtitle.waiting with English “Waiting” and Japanese “待機中”.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • #1027: Sidebar 'Running' and 'Needs Input' status indicators are flaky and unreliable

"A rabbit hops where turnIds drift and play,
Same-session clears sweep stale badges away;
If sessions match and input waits forlorn,
A tidy idle or running is gently reborn.
Separate sessions keep their own light—no stray! 🐇"


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Cmux Full Internationalization ❌ Error New localization key "agent.claude.input.subtitle.waiting" introduced for user-facing Swift string but missing translations for 18 of 20 supported locales (only en/ja present). Add translated entries for all 18 missing locales (ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, zh-Hant) to the "agent.claude.input.subtitle.waiting" key in Resources/Localizable.xcstrings.
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Cmux Architecture Rethink ❓ Inconclusive No result was produced after verification. Marking as INCONCLUSIVE. Re-run the check or adjust instructions to produce a final result.
✅ Passed checks (16 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main fix: clearing a stuck 'Needs input' state when follow-up events have drifted turnIds, directly addressing issue #1027.
Description check ✅ Passed The description is comprehensive, covering what changed (new clearing gate and test cases), why (root cause of turn drift), testing approach (three integration tests + local validation), and checklist. All major template sections are addressed.
Linked Issues check ✅ Passed The PR directly addresses #1027's requirements: ensures 'Needs Input' clears promptly even with turn drift (same session), preserves cross-session safety, and implements atomic state transitions via the new clearing gate.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the stuck 'Needs input' clearing issue via the new isCurrentOrClearsStaleNeedsInput gate and three regression tests. The localization string addition for 'Waiting' subtitle supports the AskUserQuestion handling refinement, which is in scope.
Cmux Swift Actor Isolation ✅ Passed PR introduces no Swift 6 actor isolation violations. New functions are private/internal, used synchronously, and don't expose shared mutable state without proper isolation.
Cmux Swift Blocking Runtime ✅ Passed No new blocking/timing synchronization introduced. The added isCurrentOrClearsStaleNeedsInput() uses pre-existing withLockedState() pattern with file-level locking.
Cmux No Hacky Sleeps ✅ Passed Rule excludes Swift files (covered by swift-blocking-runtime.md), applies only to TypeScript/JS/shell/build scripts. PR contains only Swift code and resource files.
Cmux Algorithmic Complexity ✅ Passed New production code uses O(1) dictionary lookups and string comparisons only; no loops, nested scans, batch rescans, or repeated filtering in hot paths. Test-only code is excluded per rules.
Cmux Swift Concurrency ✅ Passed The #1027 fix code avoids legacy async patterns. Other NSLock/DispatchQueue.global usage is justified by third-party and OS-level boundaries or test-only synchronization, all within rule exceptions.
Cmux Swift @Concurrent ✅ Passed Both new functions are synchronous, not async, so @concurrent annotations not applicable and no rule violations found.
Cmux Swift File And Package Boundaries ✅ Passed PR adds ~100 lines to oversized cmux.swift for bug fix #1027. Allowed exception: existing file touched incidentally with focused change and clear extraction path.
Cmux Swift Logging ✅ Passed New functions use no print/debugPrint/dump/NSLog. Telemetry breadcrumbs properly use CLISocketSentryTelemetry without exposing secrets. CLI hook responses allowed. No logging violations in tests.
Cmux User-Facing Error Privacy ✅ Passed Localized "Waiting" string is generic UI term. Clearing-gate-error breadcrumb is internal telemetry only. No vendor names, credentials, or implementation details exposed to users.
Cmux Swiftui State Layout ✅ Passed PR modifies only CLI backend code and tests. No SwiftUI View definitions, @Published, @StateObject, @EnvironmentObject, or layout changes introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR contains no new or materially changed standalone cmux-owned windows; changes are Claude agent lifecycle logic, tests, and localization strings.
Cmux Source Artifacts ✅ Passed All three changed files are intentional source, test, or localization catalogs allowed per source-control-artifacts.md PASS criteria; no artifacts found.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@RubiconPerform

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Jun 8, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@RubiconPerform I can't start this review because your workspace has reached its free monthly review limit. Reviews resume at the start of your next billing cycle. Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the PR diff
gh pr view 5666 --repo manaflow-ai/cmux --json files,additions,deletions,changedFiles
echo "---DIFF---"
gh pr diff 5666 --repo manaflow-ai/cmux

Length of output: 17418



@RubiconPerform — solid fix with a well-contained scope. The relaxed gate is correctly limited to same-session + needsInput state, the setting paths keep the strict guard, and the cross-session guardrail test mirrors the existing testClaudeStopFromPreviousSessionDoesNotClobberClearRunningStatus. A few things worth looking at:


🟡 Missing regression test for prompt-submit turn-drift

prompt-submit is listed as one of the three clearing transitions in the PR description, but there is no test for the turn-drift case analogous to the stop and pre-tool-use tests. Given the pattern of the other two tests, this should be straightforward to add:

func testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftPromptSubmit() throws {
    let context = try makeClaudeHookContext(name: "claude-needsinput-clear-promptsubmit")
    defer { context.cleanup() }

    let sessionId = "stuck-needsinput-promptsubmit-session"

    // Establish turn-1 as the active turn
    let promptSubmit1 = runClaudeHook(
        context: context,
        arguments: ["hooks", "claude", "prompt-submit"],
        standardInput: #"{"session_id":"\#(sessionId)","turn_id":"turn-1","cwd":"\#(context.root.path)","hook_event_name":"UserPromptSubmit","prompt":"continue"}"#
    )
    XCTAssertEqual(promptSubmit1.status, 0, promptSubmit1.stderr)

    // Notification leaves the session on needsInput with active turn still at turn-1
    let notification = runClaudeHook(
        context: context,
        arguments: ["hooks", "claude", "notification"],
        standardInput: #"{"session_id":"\#(sessionId)","cwd":"\#(context.root.path)","hook_event_name":"Notification","message":"Claude needs your permission"}"#
    )
    XCTAssertEqual(notification.status, 0, notification.stderr)
    XCTAssertEqual(
        try readClaudeHookSession(sessionId, context: context)["agentLifecycle"] as? String,
        "needsInput"
    )

    let cmdStart = context.state.commands.count
    // Same session, drifted turn id
    let promptSubmit2 = runClaudeHook(
        context: context,
        arguments: ["hooks", "claude", "prompt-submit"],
        standardInput: #"{"session_id":"\#(sessionId)","turn_id":"turn-2","cwd":"\#(context.root.path)","hook_event_name":"UserPromptSubmit","prompt":"yes"}"#
    )
    XCTAssertEqual(promptSubmit2.status, 0, promptSubmit2.stderr)

    XCTAssertEqual(
        try readClaudeHookSession(sessionId, context: context)["agentLifecycle"] as? String,
        "running",
        "A same-session prompt-submit with a drifted turn id must clear the stale needsInput to running"
    )
    let cmds = Array(context.state.commands.dropFirst(cmdStart))
    XCTAssertTrue(
        cmds.contains { $0.hasPrefix("set_agent_lifecycle claude_code running --tab=\(context.workspaceId)") },
        "Drifted prompt-submit must emit running lifecycle, saw \(cmds)"
    )
}

🟡 AskUserQuestion question-text gating is a new behavioral change

Before this PR, the AskUserQuestion branch inside pre-tool-use stored the question text whenever parsedInput.sessionId != nil, regardless of isCurrent. Now it additionally requires shouldApplyClaudeHookVisibleMutation(...) to be true:

// Before (implicit): question text stored whenever sessionId != nil
if let toolName = ...,
   toolName == "AskUserQuestion",
   let question = ...,
   let sessionId = parsedInput.sessionId { ... }

// After: question text also gated by the strict isCurrent check
   let sessionId = parsedInput.sessionId,
   shouldApplyClaudeHookVisibleMutation(...) { ... }

In the happy path this is fine, but consider a race where a pre-tool-use(AskUserQuestion) arrives with a drifted turnId (e.g., after a rapid resume). The strict gate would block storing the question text, so the subsequent Notification would fall back to the generic "Claude Code needs your attention" message instead of the actual question. If that's acceptable, it's worth an explicit comment; if not, the question-text storage could remain ungated while only the needsInput-setting mutation is gated:

// Option: always store question text, only gate the needsInput mutation
if let toolName = ...,
   toolName == "AskUserQuestion",
   let question = ...,
   let sessionId = parsedInput.sessionId {
    // Store question text unconditionally (used by Notification for display only)
    sessionStore.setQuestionText(question, for: sessionId)
    
    // Only mutate agentLifecycle when current
    if shouldApplyClaudeHookVisibleMutation(...) {
        sessionStore.setLifecycle(.needsInput, for: sessionId, workspaceId: workspaceId)
    }
}

🟢 Two lock acquisitions in isCurrentOrClearsStaleNeedsInput

isCurrent(...) and the fallback withLockedState { ... } each acquire the lock independently. This is fine for correctness (the result is used as an optimistic hint, not a critical section), but there's a theoretical TOCTOU window between the two calls if agentLifecycle transitions off needsInput concurrently. Given the existing lock structure this is probably acceptable, but worth a comment noting the two-phase read is intentional.


✅ Things that look correct

  • The cross-session guardrail (active.sessionId == normalizedSessionId) is tight and correctly prevents a different-session event from clearing the active workspace's badge.
  • shouldApplyClaudeHookClearingMutation error handling (return true) is consistent with the existing shouldApplyClaudeHookVisibleMutation pattern — failing open for clearing events is less risky than failing open for setting events.
  • The normalizeOptional(workspaceId) call on a non-optional String is consistent with the existing isCurrent guard pattern.
  • If isCurrent throws, isCurrentOrClearsStaleNeedsInput propagates the throw; shouldApplyClaudeHookClearingMutation catches it and returns true. This is the correct fail-open path for a clearing gate.
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed5db2578e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CLI/cmux.swift Outdated
@greptile-apps

greptile-apps Bot commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes the stuck "Needs input" sidebar badge (issue #1027) by introducing isCurrentOrClearsStaleNeedsInput — a single-lock gate that relaxes the strict isCurrent check for the three clearing transitions (stop, prompt-submit, pre-tool-use) when the event is for the same active session but its turnId has drifted after a Notification. Cross-session events continue to fail closed.

  • New gate is atomic: the previous double-withLockedState TOCTOU issue is resolved; isCurrent logic is fully inlined into one lock acquisition so the session-ID and needsInput checks can't race.
  • AskUserQuestion restructured: handled before the relaxed clearing gate with a strict gate and an early return, so a drifted AskUserQuestion can never fall through to the running upsert and wipe a live "Needs input" badge.
  • Five regression tests added: cover all three drifted clearing transitions, the cross-session guardrail, and the new AskUserQuestion-drift guardrail; closes coverage gaps flagged in earlier review rounds.

Confidence Score: 5/5

The change is safe to merge; the gate logic is atomic, the cross-session guardrail is preserved, and all three clearing transitions now have regression coverage including the previously missing prompt-submit and AskUserQuestion-drift cases.

The atomicity issue (double withLockedState) and the AskUserQuestion fall-through regression identified in earlier rounds are both fully addressed. The new gate is self-contained and the test suite explicitly covers the same-session/drifted, cross-session, and AskUserQuestion-drift scenarios. The only remaining gap — partial locale coverage in the string catalog — is consistent with the established pattern for the entire agent.* namespace and does not affect functionality.

No files require special attention for merging; the Localizable.xcstrings locale gap is a follow-up quality item.

Important Files Changed

Filename Overview
CLI/cmux.swift Adds isCurrentOrClearsStaleNeedsInput (single-lock, atomically inlines the isCurrent conditions) and shouldApplyClaudeHookClearingMutation wrapper; reroutes stop, prompt-submit, and non-AskUserQuestion pre-tool-use to the relaxed gate; restructures pre-tool-use so AskUserQuestion is handled with the strict gate first and returns early — preventing drifted AskUserQuestion events from falling through to the running path. All previously flagged issues (double lock, duplicate breadcrumb key, AskUserQuestion regression) are resolved in this revision.
Resources/Localizable.xcstrings Adds agent.claude.input.subtitle.waiting with en and ja translations, properly replacing the previously hardcoded "Waiting" string. Consistent with the existing en+ja-only pattern in the agent.* namespace, but missing 14+ locales that the catalog supports for other surfaces.
cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift Adds five regression tests covering all three clearing transitions (stop, pre-tool-use, prompt-submit) with drifted turn IDs, the cross-session guardrail, and the AskUserQuestion non-clearing guardrail. Closes the previously flagged prompt-submit and AskUserQuestion drift coverage gaps.

Sequence Diagram

sequenceDiagram
    participant C as Claude CLI
    participant G as Gate (shouldApplyClaudeHookClearingMutation)
    participant S as ClaudeHookSessionStore
    participant UI as Sidebar Badge

    Note over C,UI: Happy path — same session, same turnId
    C->>G: "stop(sessionId=S1, turnId=T1)"
    G->>S: isCurrentOrClearsStaleNeedsInput (single withLockedState)
    S-->>G: "active.sessionId==S1, activeTurnId==T1 → true"
    G-->>C: allow
    C->>UI: set_agent_lifecycle idle

    Note over C,UI: Bug 1027 — same session, drifted turnId, needsInput
    C->>G: "stop(sessionId=S1, turnId=T2)"
    G->>S: isCurrentOrClearsStaleNeedsInput (single withLockedState)
    S-->>G: "active.sessionId==S1, activeTurnId≠T2, lifecycle==needsInput → true"
    G-->>C: allow (relaxed gate)
    C->>UI: set_agent_lifecycle idle — badge cleared

    Note over C,UI: Guardrail — different session
    C->>G: "stop(sessionId=S2, turnId=T9)"
    G->>S: isCurrentOrClearsStaleNeedsInput (single withLockedState)
    S-->>G: "active.sessionId==S1 ≠ S2 → false"
    G-->>C: block
    C->>UI: no mutation — S1 stays on needsInput
Loading

Reviews (6): Last reviewed commit: "Address review: localize the Claude "Wai..." | Re-trigger Greptile

Comment thread CLI/cmux.swift
Comment thread CLI/cmux.swift
Comment on lines 22362 to 22399
}
}

/// Gate for Claude CLEARING transitions only. Identical to
/// `shouldApplyClaudeHookVisibleMutation` except it also applies when the SAME active session
/// is stuck on `.needsInput` after the follow-up event's `turnId` drifted, so a stale
/// "Needs input" badge is cleared instead of being stranded. A different-session event still
/// fails closed via `isCurrentOrClearsStaleNeedsInput`. See
/// https://github.com/manaflow-ai/cmux/issues/1027.
private func shouldApplyClaudeHookClearingMutation(
sessionStore: ClaudeHookSessionStore,
parsedInput: ClaudeHookParsedInput,
workspaceId: String,
telemetry: CLISocketSentryTelemetry
) -> Bool {
do {
return try sessionStore.isCurrentOrClearsStaleNeedsInput(
sessionId: parsedInput.sessionId,
workspaceId: workspaceId,
turnId: parsedInput.turnId
)
} catch {
telemetry.breadcrumb(
"claude-hook.is-current.error",
data: [
"error": String(describing: error),
"session_id": parsedInput.sessionId ?? "",
"workspace_id": workspaceId,
"turn_id": parsedInput.turnId ?? "",
]
)
return true
}
}

private func shouldReplaceStoppedClaudeSession(
sessionStore: ClaudeHookSessionStore,
parsedInput: ClaudeHookParsedInput,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Symptom patch that leaves the bad state representable

The root cause is that the Notification handler sets needsInput but does not advance active.turnId, so every subsequent event arrives with a drifted turn ID and the strict gate drops it. This PR introduces a new side-channel gate (isCurrentOrClearsStaleNeedsInput) and a parallel wrapper (shouldApplyClaudeHookClearingMutation) that work around the dropped mutations after the fact. A minimal alternative that names the invariant: in the Notification handler, carry forward the event's turnId so active.turnId advances to match, keeping isCurrent as the single gate for all transitions.

Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)

Comment thread CLI/cmux.swift
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a relaxed clearing gate (isCurrentOrClearsStaleNeedsInput) used only by the three lifecycle-clearing hook events (stop, prompt-submit, pre-tool-use), so a same-session follow-up event whose turnId drifted after a Notification can still clear a stale "Needs input" badge. The needsInput-setting paths keep the strict isCurrent gate.

  • ClaudeHookSessionStore.isCurrentOrClearsStaleNeedsInput delegates first to isCurrent and on failure makes a second withLockedState check for agentLifecycle == .needsInput; this two-lock pattern is non-atomic.
  • shouldApplyClaudeHookClearingMutation mirrors shouldApplyClaudeHookVisibleMutation but calls the relaxed gate; the AskUserQuestion branch gains its own strict guard.
  • Three integration regression tests are added; the prompt-submit turn-drift path is not yet covered.

Confidence Score: 3/5

The different-session guard is correct, but the new isCurrentOrClearsStaleNeedsInput acquires flock twice non-atomically, leaving a race window where concurrent hook processes can observe inconsistent state.

The new isCurrentOrClearsStaleNeedsInput acquires the file lock twice non-atomically — isCurrent takes and releases flock, then the caller takes it again for the agentLifecycle check. A concurrent hook process can write between those two acquisitions, making the gate read stale state and potentially allow or block a clearing transition incorrectly.

CLI/cmux.swift — specifically the new isCurrentOrClearsStaleNeedsInput method and its double-lock pattern.

Important Files Changed

Filename Overview
CLI/cmux.swift Adds isCurrentOrClearsStaleNeedsInput and shouldApplyClaudeHookClearingMutation to relax the clearing gate for stop/prompt-submit/pre-tool-use; has a non-atomic double-flock pattern and leaves the turn-drift root cause unaddressed.
cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift Adds three integration regression tests covering same-session turn-drift clearing (stop → idle, pre-tool-use → running) and the different-session guardrail; missing a test for the prompt-submit turn-drift path.

Reviews (1): Last reviewed commit: "Fix #1027: clear a stuck "Needs input" w..." | Re-trigger Greptile

Comment thread CLI/cmux.swift
Comment thread CLI/cmux.swift Outdated
Comment thread cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
@greptile-apps

greptile-apps Bot commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the "Needs input" badge getting permanently stuck in the sidebar by introducing a relaxed clearing gate (isCurrentOrClearsStaleNeedsInput) that allows stop, prompt-submit, and pre-tool-use events to clear a stale needsInput state even when the event's turnId has drifted from the workspace's active turn — a condition caused by Notification events not advancing the active turn pointer.

  • New gate function ClaudeHookSessionStore.isCurrentOrClearsStaleNeedsInput returns true for same-session events when the session is stuck on .needsInput, bypassing the strict turnId check; different-session events still fail closed.
  • Three clearing handlers (stop, prompt-submit, pre-tool-use) are switched from shouldApplyClaudeHookVisibleMutation to the new shouldApplyClaudeHookClearingMutation wrapper; the AskUserQuestion branch in pre-tool-use retains the strict gate.
  • Three integration tests validate same-session turn-drift clearing for stop and pre-tool-use, plus the different-session guardrail; a prompt-submit turn-drift test is missing.

Confidence Score: 3/5

The relaxed clearing gate introduces a regression in the pre-tool-use handler: when Claude follows a Notification with an AskUserQuestion in a new turn, the stale-needsInput fallback fires the clearing path and the badge incorrectly transitions to Running instead of Needs input, a state the sidebar cannot recover from without another user interaction.

The stop and prompt-submit paths are straightforward and the relaxation is safe there. The pre-tool-use path is more complex because it has two diverging outcomes depending on the tool name. The clearing gate is applied before the tool-name check, so a drifted-turnId AskUserQuestion event passes the clearing guard, the strict AskUserQuestion branch is then skipped, and lifecycle lands on .running. Because the following Notification is also gated out by the strict check, the badge is stranded at Running — worse than the pre-fix stuck-needsInput behavior.

CLI/cmux.swift — specifically the pre-tool-use handler around the AskUserQuestion branch (lines 22110–22169) where the new clearing gate and the strict AskUserQuestion gate interact.

Important Files Changed

Filename Overview
CLI/cmux.swift Adds isCurrentOrClearsStaleNeedsInput and shouldApplyClaudeHookClearingMutation to relax the clearing gate for stop/prompt-submit/pre-tool-use; the pre-tool-use handler has a regression where a drifted-turnId AskUserQuestion event falls through to .running instead of .needsInput
cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift Adds three integration regression tests covering same-session turn-drift clearing (stop and pre-tool-use) and the different-session guardrail; missing a companion test for the prompt-submit drifted-turnId path

Sequence Diagram

sequenceDiagram
    participant U as User
    participant CH as Claude Hook CLI
    participant SS as ClaudeHookSessionStore
    participant UI as Sidebar Badge

    Note over CH,UI: Bug scenario (pre-fix): turnId drift strands Needs input
    U->>CH: prompt-submit (turn-1)
    CH->>SS: upsert(turn-1, running)
    CH->>UI: running

    CH->>SS: notification → needsInput (turn stays turn-1)
    CH->>UI: Needs input

    CH->>SS: stop(turn-2, same session)
    SS-->>CH: "isCurrent=false (turn-2 ≠ turn-1)"
    Note over CH: gated out — badge stranded

    Note over CH,UI: Fix: isCurrentOrClearsStaleNeedsInput relaxes clearing gate
    CH->>SS: stop(turn-2, same session)
    SS-->>CH: "isCurrent=false, but needsInput+same session → true"
    CH->>SS: upsert(idle)
    CH->>UI: idle

    Note over CH,UI: Regression: pre-tool-use AskUserQuestion + drifted turnId
    CH->>SS: pre-tool-use(AskUserQuestion, turn-2)
    SS-->>CH: clearing gate passes (needsInput fallback)
    Note over CH: strict AskUserQuestion check fails (turn-2 ≠ turn-1)
    CH->>SS: upsert(running) falls through
    CH->>UI: Running (wrong)
    CH->>SS: notification(turn-2) — strict gate fails, gated out
    Note over UI: Badge stranded at Running
Loading

Comments Outside Diff (1)

  1. CLI/cmux.swift, line 22110-22169 (link)

    P1 pre-tool-use AskUserQuestion regression on drifted turnId

    When the session is stuck on .needsInput (e.g. from a prior Notification) and Claude fires a pre-tool-use for AskUserQuestion in a new turn (drifted turnId), the top-level guard shouldApplyClaudeHookClearingMutation now passes via the stale-needsInput fallback. The AskUserQuestion branch then checks shouldApplyClaudeHookVisibleMutation (strict isCurrent) which returns false for the drifted turn, so that branch is skipped and execution falls through to the generic .running upsert at line 22178. The session transitions to .running and the subsequent Notification (which also carries the drifted turnId) is gated out by its own strict shouldApplyClaudeHookVisibleMutation check. The net result is the badge stranded at "Running" — arguably worse than the pre-fix "Needs input" stuck state, because the user loses the prompt to interact.

    Concretely: after denying a Bash permission, Claude could start a new turn and ask a follow-up question via AskUserQuestion. The pre-tool-use for that question arrives with turn-2, the active turn is still turn-1 (Notification didn't advance it), the clearing gate fires, the strict AskUserQuestion check fails, and the sidebar shows "Running" while Claude is waiting for user input.

    The simplest fix is to check for AskUserQuestion before the top-level clearing gate, so that the strict gate is applied first and the function returns without entering the clearing path at all.

    Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)

Reviews (3): Last reviewed commit: "Fix #1027: clear a stuck "Needs input" w..." | Re-trigger Greptile

Comment thread CLI/cmux.swift
Comment thread cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CLI/cmux.swift`:
- Around line 1227-1246: The helper isCurrentOrClearsStaleNeedsInput currently
calls isCurrent(...) outside the withLockedState lock then reopens the store to
re-check active session state, allowing a race; instead, acquire the same
withLockedState once and evaluate both predicates atomically: perform the
normalizedSessionId/normalizedWorkspace checks and call the in-lock equivalent
of isCurrent (i.e., check whether the provided sessionId/workspaceId/turnId
match the current active session/turn) and also check
state.sessions[normalizedSessionId]?.agentLifecycle == .needsInput inside that
same locked snapshot; update the function so all reads (normalizeOptional usage,
activeSessionsByWorkspace lookup, session lifecycle check, and any current-turn
comparison) happen within a single withLockedState closure to prevent races.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c05fd7c9-e2ac-4824-baa7-e4907957ee51

📥 Commits

Reviewing files that changed from the base of the PR and between ee22255 and ed5db25.

📒 Files selected for processing (2)
  • CLI/cmux.swift
  • cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift

Comment thread CLI/cmux.swift
Codex review flagged that with the relaxed pre-tool-use clearing gate, a
PreToolUse for AskUserQuestion whose turnId had drifted (while the same active
session was already on needsInput) passed the relaxed entry guard, failed the
inner strict gate, and then fell through to the generic path that upserts
.running -- so an actual user-question event wiped the live "Needs input".

Restructure the pre-tool-use handler so AskUserQuestion is handled as its own
needsInput-SETTING event BEFORE the relaxed clearing gate and always returns.
Raising needsInput keeps the strict gate; a stale/drifted AskUserQuestion now
leaves state as-is instead of clearing it. The relaxed clearing gate now only
governs non-AskUserQuestion pre-tool-use (Claude resuming work -> running).

Also addressing CodeRabbit:
- Add the missing prompt-submit turn-drift regression test (the third clearing
  transition, alongside the stop and pre-tool-use tests).
- Add a regression test asserting a drifted AskUserQuestion does NOT clear
  needsInput (locks in the codex fix).
- Document the intentional two-phase read in isCurrentOrClearsStaleNeedsInput.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@RubiconPerform

Copy link
Copy Markdown
Contributor Author

Pushed 076c85f9a addressing the review feedback — thanks both.

@codex (P2 — a drifted AskUserQuestion could clear "Needs input"): Confirmed, good catch, and fixed. AskUserQuestion is a needsInput-setting event, so it's now handled before the relaxed clearing gate and always returns. A drifted/stale AskUserQuestion therefore leaves state as-is instead of falling through to the generic .running path and wiping a live "Needs input". Raising needsInput keeps the strict isCurrent gate; the relaxed clearing gate now governs only non-AskUserQuestion pre-tool-use (Claude resuming work → running). Added testClaudeAskUserQuestionWithDriftedTurnDoesNotClearNeedsInput to lock it in.

@coderabbitai:

  • Added the missing prompt-submit turn-drift test — testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftPromptSubmit (the third clearing transition, alongside the stop and pre-tool-use cases).
  • Documented the intentional two-phase read in isCurrentOrClearsStaleNeedsInput (optimistic gate hint, not a critical section — a benign TOCTOU there can only ever clear a stale badge, never the reverse).

Both changed files are swiftc -parse clean. The full cmux-unit suite still needs Xcode 26 (I only have Xcode 15 locally), so I'm relying on CI to run the integration tests.

@codex review
@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr diff 5666 --repo manaflow-ai/cmux

Length of output: 26903



@RubiconPerform — all three items verified in the diff. Everything looks correct:

  1. prompt-submit turn-drift test ✅ — testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftPromptSubmit is present and follows the same structure as the stop and pre-tool-use companions.

  2. AskUserQuestion early-return ✅ — Moving it before the relaxed gate and always returning is cleaner than the "option" I sketched. The else branch emits claude-hook.pre-tool-use.ask-user-question.skipped for observability on the drifted/not-current path. testClaudeAskUserQuestionWithDriftedTurnDoesNotClearNeedsInput locks it in as a regression test.

  3. Two-phase read comment ✅ — The inline comment in isCurrentOrClearsStaleNeedsInput is precise: "A benign TOCTOU can at most let a clearing event proceed against a needsInput that just changed — which only ever clears a stale badge, never the reverse."

The five new tests collectively cover all three clearing transitions under turn drift, the cross-session guardrail, and the AskUserQuestion non-clearing invariant. Looks good to merge pending CI.

🐇 ✨

[resolve]

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 076c85f9af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CLI/cmux.swift
// is cleared even when this event's turnId drifted from the active turn
// (https://github.com/manaflow-ai/cmux/issues/1027). A different-session event still
// fails closed.
guard shouldApplyClaudeHookClearingMutation(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update active turn after drifted pre-tool-use

When this relaxed gate admits a same-session pre-tool-use whose turnId drifted, the handler clears the lifecycle to running but never advances activeSessionsByWorkspace to parsedInput.turnId (unlike prompt-submit, which upserts with markActive: true and the new turn). In the repaired flow prompt-submit(turn-1) -> Notification(needsInput) -> pre-tool-use(turn-2), the active turn remains turn-1, so a later strict-gated Notification or AskUserQuestion for turn-2 is treated as stale and the next permission/question prompt will not surface.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
CLI/cmux.swift (1)

1239-1245: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the relaxed clearing gate atomic.

Line 1239 still leaves a race between isCurrent(...) and the second withLockedState read. If a newer same-session needsInput update lands in that gap, an older CLEARING event can still pass here and clear a live badge, which reintroduces the false-negative this PR is meant to remove. Evaluate the current-turn check and the stale-clear fallback from one locked snapshot.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLI/cmux.swift` around lines 1239 - 1245, The relaxed gate currently has a
TOCTOU between isCurrent(...) and the later withLockedState closure, allowing an
older CLEARING event to clear a live needsInput badge; fix this by performing
the current-turn check and the stale-clear fallback inside a single
withLockedState snapshot so both
activeSessionsByWorkspace[normalizedWorkspace].sessionId == normalizedSessionId
and the needsInput/CLEARING logic are evaluated atomically; specifically, move
the isCurrent evaluation into the same withLockedState closure (or fetch the
same-session marker from state once and use it for both the current-turn and
stale-clear decisions) so decisions around activeSessionsByWorkspace,
normalizedSessionId and needsInput are based on one locked snapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CLI/cmux.swift`:
- Around line 22131-22145: Replace the hard-coded "Waiting" subtitle passed to
sessionStore.upsert with a localized string key; specifically, change the value
assigned to lastSubtitle in the call to sessionStore.upsert (the code block
using sessionStore.upsert and the lastSubtitle parameter) to use
String(localized: "CLAUDE_AGENT_WAITING_SUBTITLE", defaultValue: "Waiting") (or
your project's localization helper) and add the key
"CLAUDE_AGENT_WAITING_SUBTITLE" with translations to
Resources/Localizable.xcstrings for all supported locales so the UI displays a
localized subtitle instead of the hard-coded English text.

---

Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 1239-1245: The relaxed gate currently has a TOCTOU between
isCurrent(...) and the later withLockedState closure, allowing an older CLEARING
event to clear a live needsInput badge; fix this by performing the current-turn
check and the stale-clear fallback inside a single withLockedState snapshot so
both activeSessionsByWorkspace[normalizedWorkspace].sessionId ==
normalizedSessionId and the needsInput/CLEARING logic are evaluated atomically;
specifically, move the isCurrent evaluation into the same withLockedState
closure (or fetch the same-session marker from state once and use it for both
the current-turn and stale-clear decisions) so decisions around
activeSessionsByWorkspace, normalizedSessionId and needsInput are based on one
locked snapshot.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 00871f95-8604-441f-919b-a703af88e6d6

📥 Commits

Reviewing files that changed from the base of the PR and between ed5db25 and 076c85f.

📒 Files selected for processing (2)
  • CLI/cmux.swift
  • cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift

Comment thread CLI/cmux.swift
Greptile (P1) + CodeRabbit (Major): isCurrentOrClearsStaleNeedsInput called
isCurrent() (one withLockedState) and then re-acquired the lock for the
needsInput check -- a non-atomic two-phase read where a concurrent session
update between the two lock windows could flip the decision. Inline the
isCurrent predicate so the turn-identity check and the needsInput check are
evaluated from a SINGLE withLockedState snapshot. This also removes the
post-isCurrent re-normalization guard that Greptile flagged as unreachable
dead code.

Greptile (P2): give shouldApplyClaudeHookClearingMutation its own
"claude-hook.clearing-gate.error" telemetry breadcrumb so its errors are
distinguishable from the strict gate's "claude-hook.is-current.error".

Behavior is unchanged (verified by the standalone gate harness across all
clear/block/no-over-clear/same-turn cases); this is a structural hardening of
the same logic.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@RubiconPerform

Copy link
Copy Markdown
Contributor Author

Pushed 05d62c5f0 addressing the latest round — thanks for the thorough pass.

Atomic gate (@greptile P1, @coderabbitai Major): isCurrentOrClearsStaleNeedsInput now inlines the isCurrent predicate into a single withLockedState snapshot, so the turn-identity check and the needsInput check are evaluated atomically and can't race a concurrent session update between two lock windows. This also removes the post-isCurrent re-normalization guard that @greptile flagged as unreachable dead code — it's gone with the refactor.

Distinct breadcrumb (@greptile P2): the clearing gate now emits claude-hook.clearing-gate.error instead of colliding with the strict gate's claude-hook.is-current.error.

Already in 076c85f9a (you were reviewing the first commit ed5db2578): the missing prompt-submit turn-drift test is added, and the AskUserQuestion fall-through (@codex P2) is fixed — it's now handled as a needsInput-setting event before the relaxed gate and always returns, so a drifted AskUserQuestion can't fall through to running and clear the badge (covered by testClaudeAskUserQuestionWithDriftedTurnDoesNotClearNeedsInput).

"Advance the active turn" / "symptom patch" (@codex P2, @greptile P2): agreed these describe the more complete model. I've kept this PR scoped to the stuck-badge root cause deliberately: (a) the turn-non-advancement is pre-existing, not introduced here; (b) advancing active.turnId on clearing events reaches into the turn-stack machinery (markActive / activePromptTurnIds) where I'd rather not risk side-effects as an outside contributor; and (c) the Notification payload that sets needsInput doesn't reliably carry a turnId to "carry forward." Happy to do the turn-advancement refactor as a follow-up if you'd prefer it over the gate approach.

Waiting subtitle (@coderabbitai): that string is pre-existing — I relocated the block, didn't add it — so I've left localizing it out of scope for this bugfix.

Verification: built and ran the cmux-unit suite locally under Xcode 16.4 / Swift 6.1 — the change compiles clean and 144 tests pass. The remaining failures in my run (including the pre-existing testClaudeStopFromPreviousSessionDoesNotClobberClearRunningStatus and ~10 unrelated SSH/Tmux/Grok tests) are a local test-host control-socket issue: surface.list goes unanswered (mobile host listener disabled; publishing XCTest routes without binding), so every socket-driven hook times out at 15s regardless of the diff. Relying on CI for the integration tests. The gate logic itself I validated with a standalone harness (same-session-drift clears, different-session blocked, non-needsInput not over-cleared, same-turn unchanged).

@codex review
@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr diff 5666 --repo manaflow-ai/cmux

Length of output: 27189



@RubiconPerform — commit 05d62c5f0 addresses all three open items cleanly:

  1. Atomic gate ✅ — isCurrentOrClearsStaleNeedsInput now evaluates the turn-identity check and the .needsInput check inside a single withLockedState closure; the prior two-phase read (and the now-unreachable re-normalization guard) are gone. The logic path is straightforward:

    • nil sessionId/workspace → fail open (consistent with isCurrent)
    • no active session for workspace → true
    • cross-session → false (guardrail intact)
    • turn match → true
    • turn drift, same session → return state.sessions[id]?.agentLifecycle == .needsInput
  2. Distinct breadcrumb ✅ — shouldApplyClaudeHookClearingMutation now emits claude-hook.clearing-gate.error, no collision with claude-hook.is-current.error.

  3. Five regression tests ✅ — all three clearing transitions under same-session drift (stop, pre-tool-use, prompt-submit), the cross-session guardrail, and the AskUserQuestion non-clearing invariant are covered. The testClaudeAskUserQuestionWithDriftedTurnDoesNotClearNeedsInput test also locks in the codex review finding from the first revision.

The AskUserQuestion refactor (early-return before the relaxed gate, skipped breadcrumb on the drift path) is a nice structural improvement over the "option" sketched earlier. Ready to merge pending CI green.

🐇 ✨

[resolve]

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

All concerns from the previous round are addressed in 05d62c5f0.

P1 items — fully resolved:

  • Atomic gate: isCurrentOrClearsStaleNeedsInput now inlines all the isCurrent conditions into a single withLockedState closure. The turn-identity check and the agentLifecycle == .needsInput check are evaluated atomically against the same snapshot — no race window between two lock acquisitions.
  • AskUserQuestion fall-through: Handled as its own branch before the relaxed clearing gate, unconditionally exits with print("OK"); return. A drifted AskUserQuestion can no longer fall through to the running path.

P2 items — resolved:

  • Distinct breadcrumb: shouldApplyClaudeHookClearingMutation now emits claude-hook.clearing-gate.error, distinct from the strict gate's key.
  • Dead code removed: The unreachable re-normalization guard is gone — the refactored single-lock implementation makes it unnecessary.
  • Missing tests: Both the prompt-submit turn-drift test (testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftPromptSubmit) and the AskUserQuestion drift guardrail (testClaudeAskUserQuestionWithDriftedTurnDoesNotClearNeedsInput) are now present. Full coverage across all three clearing transitions + the two non-clearing guardrails.

Remaining P2 "symptom patch" concern: The reasoning is sound — Notification payloads don't reliably carry a turnId to advance the active turn, and touching markActive/activePromptTurnIds from outside the core turn-stack would carry unquantified side-effect risk. A follow-up to advance active.turnId on clearing events would be the cleaner long-term approach, but this PR is correctly scoped to the observable regression.

The implementation is clean, atomic, and well-tested. LGTM. ✅

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05d62c5f0c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CLI/cmux.swift
Comment on lines +1255 to +1256
// Same session, but the turnId drifted: still clear a stale "Needs input" badge.
return state.sessions[normalizedSessionId]?.agentLifecycle == .needsInput

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve turn guard for older same-session events

When the current Claude session is waiting for input on a newer active turn, a late hook from an older turn in the same session now passes this relaxed gate solely because the session lifecycle is .needsInput. For example, with active turn-2 showing a permission prompt, a delayed Stop for turn-1 will be treated as current and the stop handler will write idle/mark the session active, clearing the real current prompt; the previous strict isCurrent check was the protection against this same-session stale-turn case. The relaxation needs some way to distinguish the intended newer drift from older stale turns before ignoring the turn mismatch.

Useful? React with 👍 / 👎.

@RubiconPerform

Copy link
Copy Markdown
Contributor Author

@austinywang (you're assigned #1027) — status summary + one design call I'd like your steer on, plus a heads-up that the Swift CI needs a maintainer to approve the workflow run on this fork PR.

Bot threads that are already addressed (re-flagged on a stale snapshot)

A few open threads are re-posts of items fixed in later commits — flagging so they're not read as still-open:

  • "Non-atomic / double withLockedState" + "stale-needsInput fallback dead code" (Greptile P1/P2 @ :1258/:1260/:1257) → fixed in 05d62c5f0: isCurrentOrClearsStaleNeedsInput now inlines the isCurrent predicate into a single withLockedState (atomic; the dead guard is gone).
  • "Duplicate breadcrumb key" (Greptile P2 @ :22416) → fixed in 05d62c5f0 (claude-hook.clearing-gate.error).
  • "prompt-submit turn-drift untested" (Greptile P2 @ :399/:403) → added in 076c85f9a (testClaudeNotificationNeedsInputClearsOnSameSessionTurnDriftPromptSubmit).
  • AskUserQuestion fall-through (codex P2) → fixed in 076c85f9a (handled as a setting event before the relaxed gate, with a regression test).

The one I want your call on — @chatgpt-codex-connector's :1256 is a real catch

The relaxed gate keys off "same session + needsInput + turn mismatch", which clears on any turn drift — including an older, late stop/pre-tool-use for a stale turn while a newer turn is the one actually showing the prompt. The strict isCurrent previously blocked that, so as written the fix trades the persistent stuck-badge for a rare transient wrong-clear on out-of-order same-session events. Two ways forward:

  1. Tighten the gate to relax only for the current/latest turn, not older stale ones — but doing that correctly needs the turn-stack semantics (activePromptTurnIds / terminalPromptTurnIds / lastPromptTurnId), which I'd rather not guess at from outside.
  2. Greptile's root-cause alternative — advance active.turnId in the Notification handler so isCurrent stays the single gate for every transition (no relaxation, so :1256 can't happen). This hinges on whether the Notification payload reliably carries the waiting turn's id — you'd know that better than I do.

I'm happy to implement whichever you prefer — (2) looks cleanest if Notification carries a turnId. Which direction do you want?

Verification

Built + ran cmux-unit locally under Xcode 16.4 / Swift 6.1: compiles clean, 144 tests pass. The remaining failures in my run (including the pre-existing testClaudeStopFromPreviousSessionDoesNotClobberClearRunningStatus and ~10 unrelated SSH/Tmux/Grok tests) are a local test-host socket issue — surface.list goes unanswered (mobile host listener disabled; publishing XCTest routes without binding), so every socket-driven hook times out regardless of the diff. So the integration tests need CI to validate, which needs a maintainer to approve the run. Gate logic itself is covered by a standalone harness.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

Replace the hard-coded "Waiting" lastSubtitle in the pre-tool-use AskUserQuestion branch with String(localized: "agent.claude.input.subtitle.waiting", defaultValue: "Waiting"), mirroring the existing agent.codex.input.subtitle.waiting convention, and add en/ja entries to Localizable.xcstrings (en=Waiting, ja=待機中).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@RubiconPerform

Copy link
Copy Markdown
Contributor Author

Localized the Waiting subtitle in 1733504a2 — String(localized: "agent.claude.input.subtitle.waiting", defaultValue: "Waiting") plus en/ja entries in Localizable.xcstrings, mirroring the existing agent.codex.input.subtitle.waiting convention. Build verified under Xcode 16.4. Thanks @coderabbitai.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@RubiconPerform — nice follow-through. Mirroring agent.codex.input.subtitle.waiting is exactly the right pattern, and adding both en and ja entries keeps the localization table complete. That ties off the last open thread from my earlier review.

🐇 🌸


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@teamleaderleo teamleaderleo added S3: minor Wrong behavior with a workaround area: sidebar The workspace sidebar: list, groups, status, reordering area: agents Agent integrations (Claude Code, Codex, ACP), agent chat, hooks, status needs a call Finished and held for a team design or product decision (see #13742 and the gallery in #15427) labels Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: agents Agent integrations (Claude Code, Codex, ACP), agent chat, hooks, status area: sidebar The workspace sidebar: list, groups, status, reordering needs a call Finished and held for a team design or product decision (see #13742 and the gallery in #15427) S3: minor Wrong behavior with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sidebar 'Running' and 'Needs Input' status indicators are flaky and unreliable

2 participants